Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #26873 - fix vsphere networks (auto)-loading #6803

Merged
merged 1 commit into from May 28, 2019

Conversation

sbernhard
Copy link
Contributor

Due to a race condition, sometimes the the failure 'uninitialized constant Fog::Compute::Vsphere::Network' happens when using the compute-resource networks API.

@theforeman-bot
Copy link
Member

Issues: #26873

@sbernhard
Copy link
Contributor Author

Thanks @timogoebel for your help on IRC!

@timogoebel
Copy link
Member

@ezr-ondrej: What do you think about this?

@sbernhard
Copy link
Contributor Author

[test]

@tbrisker
Copy link
Member

[test foreman] [test katello]

@sbernhard
Copy link
Contributor Author

I guess the failed tests are not related to my change.

Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like it, because it is not solving the root cause, but I haven't been able to come with a proper fix, so as long as it fixes the issue I would go with it!
Just add another one to solve similar problem please.

app/models/concerns/fog_extensions.rb Show resolved Hide resolved
@ezr-ondrej
Copy link
Member

And thanks @sbernhard for issuing that! I haven't though about such simple solution 🤷‍♂️

Due to a race condition, sometimes the the failure 'uninitialized constant Fog::Compute::Vsphere::Network' happens when using the compute-resource networks API.

Similar happend to Fog::Compute::Vsphere::Cluster
@sbernhard
Copy link
Contributor Author

Thanks @ezr-ondrej. It wasn't my idea to fix it this way - its @timogoebel :-) I did just run some tests.

Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, in the end simple fix is a good fix 👍 ... I just wonder if those problems are going to be solved by puma (I hope so)

Copy link
Member

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sbernhard!

@tbrisker tbrisker merged commit 6d78db2 into theforeman:develop May 28, 2019
@tbrisker
Copy link
Member

1.22 - 8dbe4c5

@ohadlevy
Copy link
Member

Yeah, in the end simple fix is a good fix ... I just wonder if those problems are going to be solved by puma (I hope so)

pretty easy to verify, our container based environment is based on puma, maybe you can just give it a try

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants